Skip to content

Decouple the session loader into reader and writer over the cache #4445

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 59 commits into
base: master
Choose a base branch
from
Draft

Decouple the session loader into reader and writer over the cache #4445

wants to merge 59 commits into from
+379 −86

Conversation

soulomoon
Copy link
Collaborator

@soulomoon soulomoon commented Nov 9, 2024 •
edited
Loading

Follow up of and already include the changes in #4439
Refactored session loader to use architecture that having multiple readers and writer over the cache .

In bigger project, HLS usually loading up slow. One of the main obstacle is that we are loading ghc options for files in strict sequential order.
I have made an attemp to improve the situation by refactoring this loading into reader and writer over cache. This way, the long laoding file won't be blocking already loaded files, making hls more responsive and smooth.

Also it eliminates session loading for the same file from being multiply requested, by lining up in an ordered set in stead of a list.

@soulomoon
Refactor session loading to manage pending files so we can batch load…
58b8b68 
… them to improve performance fix haskell#4381
@soulomoon
distribute errors to all pending files are being loading
ea002d7
@soulomoon
better filter loading files
c78b197
@soulomoon
fallback to non-batch load
b879375
@soulomoon
typo
8953aec
@soulomoon
update inline doc
4bdc2c8
@soulomoon
update inline doc
c4bb53a
@soulomoon
add LogSessionReloadOnError to log errors during file reloads; cleanu…
112bc95 
…p error loading and cradle files
@soulomoon
Merge branch 'master' into batch-load
9a2aaf1
@soulomoon
refactor loadSessionWithOptions to improve error handling and clarify…
6e04d28 
… variable names
@soulomoon
refactor loadSessionWithOptions to improve pending file handling and …
67aebc4 
…error management
@soulomoon
add doc about limitation
98ae446
@soulomoon
absolute file at the beginning
f3eb580
@soulomoon
run session loader and worker in sperate
e7bd3d4
@soulomoon
cleanup
1f97c40
@soulomoon
rename LogGetSessionRetry to LogLookupSessionCache for clarity in log…
4c998bd 
…ging
@soulomoon soulomoon mentioned this pull request Nov 9, 2024
Open
@soulomoon soulomoon force-pushed the batch-load-multi-read branch from ba7379b to 79a43a0 Compare November 9, 2024 12:06
@soulomoon soulomoon self-assigned this Mar 6, 2025
@soulomoon
Copy link
Collaborator Author

soulomoon commented Mar 6, 2025 •
edited
Loading

I don't understand why it fails to build for ghc version before 9.10.1.
The only dependency change is adding:

source-repository-package
  type: git
  location: https://github.com/haskell/hie-bios
  tag: 65df091a4464b43043661d70c1dcfed1ed1fa9a9

@fendor Can you help me on this ? I am puzzled.

ghcup run --ghc 9.8.4 -- cabal build

give me

❯ ghcup run --ghc 9.8.4 -- cabal build
[ Info  ] downloading: https://raw.githubusercontent.com/haskell/ghcup-metadata/master/ghcup-0.0.9.yaml as file /Users/ares/.ghcup/cache/ghcup-0.0.9.yaml
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
Resolving dependencies...
Error: [Cabal-7107]
Could not resolve dependencies:
[__0] trying: ghcide-2.9.0.1 (user goal)
[__1] trying: optparse-applicative-0.18.1.0 (dependency of ghcide)
[__2] trying: base-4.19.2.0/installed-1e06 (dependency of ghcide)
[__3] trying: haskell-language-server-2.9.0.1 (user goal)
[__4] trying: haskell-language-server:+fourmolu
[__5] trying: fourmolu-0.16.2.0 (dependency of haskell-language-server +fourmolu)
[__6] next goal: ghc-lib-parser (dependency of fourmolu)
[__6] rejecting: ghc-lib-parser; 9.10.1.20241103, 9.10.1.20240511
      (constraint from cabal.project requires ==9.8.4.20241130)
[__6] rejecting: ghc-lib-parser-9.8.4.20241130 (conflict: fourmolu => ghc-lib-parser>=9.10 && <9.11)
[__6] skipping: ghc-lib-parser; 9.8.3.20241103, 9.8.3.20241022, 9.8.2.20240223, 9.8.1.20231121, 9.8.1.20231009, 9.6.6.20240701, 9.6.5.20240423, 9.6.4.20240109, 9.6.3.20231121, 9.6.3.20231014, 9.6.2.20231121, 9.6.2.20230523, 9.6.1.20230312, 9.4.8.20231111, 9.4.7.20230826, 9.4.6.20230808, 9.4.5.20230430, 9.4.4.20221225, 9.4.3.20221104, 9.4.2.20220822, 9.4.1.20220807, 9.2.8.20230729, 9.2.7.20230228, 9.2.6.20230211, 9.2.5.20221107, 9.2.4.20220729, 9.2.3.20220709, 9.2.3.20220527, 9.2.2.20220307, 9.2.1.20220109, 9.2.1.20211101, 9.2.1.20211030, 9.0.2.20211226, 9.0.1.20210324, 9.0.1.20210207, 9.0.1.20210205, 8.10.7.20220219, 8.10.7.20210828, 8.10.6.20210814, 8.10.5.20210606, 8.10.4.20210206, 8.10.3.20201220, 8.10.2.20200916, 8.10.2.20200808, 8.10.1.20200916, 8.10.1.20200523, 8.10.1.20200518, 8.10.1.20200412, 8.10.1.20200324, 8.8.4.20210620, 8.8.4.20200716, 8.8.3.20210620, 8.8.3.20200412.1, 8.8.3.20200412, 8.8.3.20200224, 8.8.2.20210620, 8.8.2.20200205, 8.8.2, 8.8.1.20210620, 8.8.1.20191204, 8.8.1, 8.8.0.20190723, 8.8.0.20190424, 0.20221201, 0.20221101, 0.20221001, 0.20220901, 0.20220801, 0.20220701, 0.20220601, 0.20220501, 0.20220401, 0.20220301, 0.20220201, 0.20220103, 0.20211201, 0.20211101, 0.20211001, 0.20210901, 0.20210801, 0.20210701, 0.20210601, 0.20210501, 0.20210331, 0.20210228, 0.20210201, 0.20210101, 0.20201201, 0.20201101, 0.20201001, 0.20200901, 0.20200801, 0.20200704, 0.20200601, 0.20200501, 0.20200401, 0.20200301, 0.20200205, 0.20200201, 0.20200102, 0.20191201, 0.20191101, 0.20191002, 0.20190909, 0.20190903, 0.20190806, 0.20190703, 0.20190603, 0.20190523, 0.20190516, 0.20190423, 0.20190402 (has the same characteristics that caused the previous version to fail: excluded by constraint '>=9.10 && <9.11' from 'fourmolu')
[__6] fail (backjumping, conflict set: fourmolu, ghc-lib-parser)
After searching the rest of the dependency tree exhaustively, these were the goals I've had most trouble fulfilling: Cabal, extensions, fourmolu, ghc-lib-parser, haskell-language-server, Cabal-syntax, optparse-applicative, base, stan, extensions:executable, haskell-language-server:fourmolu, haskell-language-server:stan, hie-bios, ghcide
Try running with --minimize-conflict-set to improve the error message.

@fendor
Copy link
Collaborator

fendor commented Mar 7, 2025 •
edited
Loading

@soulomoon That was a tricky one!

The ultimate conflict is that exe:hie-bios specifies , optparse-applicative >= 0.18.1 && < 0.19 in the executable. I suspect, that including hie-bios via source-repository-package includs the executable, which the normal dependency doesn't.

Then, the latest version of extensions (dep of stan) compatible with certain Cabal versions is https://hackage.haskell.org/package/extensions-0.1.0.1 which forces: optparse-applicative (>=0.15 && <0.18), which causes the conflict.

Adding to the cabal.project file

allow-older: hie-bios:optparse-applicative

works for me.
Maybe we want to relax the lower bound on optparse-applicative in hie-bios as well, but that's optional, since once we release the next hie-bios version, we won't have to depend on exe:hie-bios any more.

@soulomoon
Copy link
Collaborator Author

soulomoon commented Mar 7, 2025 •
edited
Loading

Thank you so much for your help @fendor.
I am wondering when would we release a new version of hie-bios, any plan on it?

@soulomoon soulomoon requested a review from Copilot April 27, 2025 14:36
Copilot
Copilot AI reviewed Apr 27, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.

Files not reviewed (5)
  • cabal.project: Language not supported
  • ghcide/ghcide.cabal: Language not supported
  • ghcide/session-loader/Development/IDE/Session.hs: Language not supported
  • ghcide/session-loader/Development/IDE/Session/Diagnostics.hs: Language not supported
  • ghcide/session-loader/Development/IDE/Session/OrderedSet.hs: Language not supported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@michaelpj michaelpj Awaiting requested review from michaelpj michaelpj will be requested when the pull request is marked ready for review michaelpj is a code owner

@wz1000 wz1000 Awaiting requested review from wz1000 wz1000 will be requested when the pull request is marked ready for review wz1000 is a code owner

@fendor fendor Awaiting requested review from fendor fendor will be requested when the pull request is marked ready for review fendor is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@soulomoon soulomoon

Labels
performance Issues about memory consumption, responsiveness, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@soulomoon @fendor